Skip to content

feat(cli): reload project runtime after /cd - #10263

Open
qqqys wants to merge 11 commits into
QwenLM:mainfrom
qqqys:feat/cd-project-runtime-reload
Open

feat(cli): reload project runtime after /cd#10263
qqqys wants to merge 11 commits into
QwenLM:mainfrom
qqqys:feat/cd-project-runtime-reload

Conversation

@qqqys

@qqqys qqqys commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This PR reloads project-scoped runtime state when an existing session changes its working directory with /cd. The switch is transactional for settings and file watching, then refreshes context files, permissions, tools, hooks, skills, subagents, MCP servers, system instructions, and durable cron scheduling for the destination project. Session-owned tools remain registered, while stale resources from the previous project are removed.

Why it's needed

Previously, /cd changed the process working directory but left runtime capabilities derived from the original project active until restart. That could expose the new project to permissions, hooks, tools, context, or background work that belonged to the previous project. Reloading the complete project runtime keeps the active session aligned with its current directory.

Reviewer Test Plan

How to verify

  1. Start a session in project A with distinct project settings, context files, permissions, hooks, skills, command tools, MCP servers, and durable cron tasks.
  2. Run /cd <project-b> where project B defines different values, and verify project-B capabilities become active without starting a new conversation.
  3. Verify project-A project-scoped tools and resources are removed while session-owned tools remain available.
  4. Verify the working-directory-change hook receives both canonical paths and the model receives the directory-change context.
  5. Try a destination with invalid project configuration and verify the switch aborts while project-A runtime state and settings watching remain active.
  6. Repeat the relocation through both the interactive UI and ACP session paths, including durable cron scheduling after the switch.

Evidence (Before & After)

Before: /cd changed the working directory while retaining project-scoped runtime capabilities from the original project.

After: the destination project runtime is loaded transactionally; blocking preparation failures roll back the switch, and non-blocking refresh failures report warnings without restoring stale capabilities.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Node.js 22 on macOS. Full build, typecheck, lint, and 4,114 focused and downstream tests passed after rebasing onto the latest upstream main.

Risk & Scope

  • Main risk or tradeoff: this touches shared session runtime infrastructure, so regressions could affect project-scoped capability refresh or cleanup after a directory change.
  • Not validated / out of scope: authentication, provider and model reconfiguration, sandbox/container relocation, and manual verification on Windows or Linux.
  • Breaking changes / migration notes: none.

Linked Issues

Closes #10173

中文说明

本 PR 做了什么

本 PR 在现有会话通过 /cd 更改工作目录时重新加载项目级运行时状态。切换过程会以事务方式更新设置和文件监听,然后刷新目标项目的上下文文件、权限、工具、Hooks、Skills、子智能体、MCP 服务、系统指令和持久化定时任务。会话级工具会继续保留,旧项目遗留的项目级资源会被移除。

为什么需要

此前 /cd 只会改变进程工作目录,启动时从原项目加载的运行时能力会一直保留到重启。这可能让新项目继续使用旧项目的权限、Hooks、工具、上下文或后台任务。完整重载项目运行时后,当前会话的能力会与当前目录保持一致。

Reviewer Test Plan

如何验证

  1. 在项目 A 启动会话,并配置有明显区别的项目设置、上下文文件、权限、Hooks、Skills、命令工具、MCP 服务和持久化定时任务。
  2. 执行 /cd <project-b>,让项目 B 提供不同配置,确认无需新建会话即可启用项目 B 的能力。
  3. 确认项目 A 的项目级工具和资源已被移除,同时会话级工具仍然可用。
  4. 确认工作目录变更 Hook 收到两个规范化路径,模型也收到目录变更上下文。
  5. 切换到包含无效项目配置的目录,确认切换被中止,项目 A 的运行时状态和设置监听保持有效。
  6. 分别通过交互界面和 ACP 会话路径验证切换,并确认切换后的持久化定时任务调度正常。

证据(修改前后)

修改前:/cd 改变工作目录后,原项目的项目级运行时能力仍然保留。

修改后:目标项目运行时以事务方式加载;阻断性的准备失败会回滚切换,非阻断刷新失败会报告警告且不会恢复旧项目能力。

测试平台

系统 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

macOS、Node.js 22。在变基到最新上游主干后,全量构建、类型检查、Lint 以及 4,114 项重点和下游测试均通过。

风险与范围

  • 主要风险或权衡:改动涉及共享的会话运行时基础设施,回归可能影响目录切换后的项目级能力刷新或资源清理。
  • 未验证或超出范围:认证、Provider 和模型重新配置,沙箱或容器迁移,以及 Windows、Linux 上的人工验证。
  • 破坏性变更或迁移说明:无。

关联 Issue

Closes #10173

@qqqys

qqqys commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

/cd project runtime reload E2E plan

Baseline

  1. Start the CLI in project A with project-specific settings, context files, permissions, hooks, skills, command tools, MCP servers, and durable cron tasks.
  2. Run /cd <project-b> where project B defines visibly different values for each capability.
  3. Confirm the existing release keeps some project-A capabilities or does not activate the project-B capabilities until restart.

Expected behavior after the change

  1. /cd <project-b> changes the working directory without starting a new conversation.
  2. Project-B settings, context files, permissions, hooks, skills, command tools, MCP servers, and durable cron tasks become active.
  3. Project-A project-scoped tools and resources are no longer available.
  4. Session-owned tools remain registered.
  5. The model receives working-directory-change context and the CwdChanged hook receives both canonical paths.
  6. Invalid project-B configuration aborts the switch and preserves the project-A runtime and settings watcher.
  7. A non-blocking refresh failure reports a warning and does not restore stale project-A capabilities.
  8. Repeat the checks through both the interactive UI and ACP session relocation paths.

Verification record

  • Unit and downstream suites cover the transactional reload, resource cleanup, session-tool preservation, custom context names, watcher retargeting, TUI scheduler restart, and ACP scheduler restart.
  • Verified after rebasing onto the latest upstream main on macOS with Node.js 22.
  • npm run build: passed.
  • npm run typecheck: passed.
  • npm run lint: passed.
  • Core focused/downstream tests: 13 files, 1,908 tests passed.
  • CLI focused/downstream tests: 12 files, 2,206 tests passed.
  • Total: 25 files, 4,114 tests passed.

@github-actions github-actions Bot added the review/self-reported The linked issue was opened by the PR author (self-reported) label Aug 27, 2026
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Deferred approval withheld — 1 PR CI workflow run(s) on 7580e80 did not finish green; see the updated table in the Stage 2 comment. Re-run @qwen-code /triage after fixes. finalize run

⚠️ 延迟审批已搁置 —— 7580e80 有 1 个 PR CI workflow 未以绿色完成,详见 Stage 2 评论中已更新的表格。修复后可重新运行 @qwen-code /triage查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

  • Template: complete ✓ — all required sections present, including the bilingual body.
  • Problem: real, not theoretical. Linked issue feat(cd): Reload project-scoped runtime configuration after /cd #10173 was verified in source during issue triage: Config.relocateWorkingDirectory() already refreshes workspace roots, memory, and MCP servers, but not settings, hooks, skills, or agent definitions — after /cd the session keeps running the previous project's hooks and permission/tool surface. Since hooks execute commands and settings control permissions, that's an actual policy-boundary gap, not a hypothetical one.
  • Direction: aligned. This fixes the semantics of an existing core command rather than adding new surface, and the issue's scope discipline is good (no credential/auth hot-swap, doesn't reopen the design(serve): Define multi-workspace session cd ownership semantics #7015 daemon-ownership question). Direct behavior reference found: Claude Code 2.1.246 changelog — "Improved /cd: the new directory's project settings, hooks, .mcp.json servers … skills, and agents now take effect right after the move instead of on --resume".
  • Size: ~1559 production lines vs ~840 test lines (2399 total across 52 files; no generated/schema files), spanning packages/core, packages/cli, and packages/acp-bridge. That's past the 1000-line large-PR advisory — worth considering a split if feasible, though the prepare/commit/rollback transaction reads most naturally as one unit, so this is informational only. For visibility: the two-tier core gate exempts maintainer-authored PRs and the author is a CODEOWNER for packages/core, so no escalation — just naming the size.
  • Approach: the prepare-and-commit design matches what the issue asked for: target settings load read-only before anything moves (a corrupt target config fails closed without touching the file), commit swaps LoadedSettings in place while the workspace watcher is paused, and chdir/realpath/artifact-migration failures roll back. Issue triage had suggested phasing (security-relevant reload first, then skills/agents/memory cleanup); this lands one-shot instead — defensible given the shared transaction, but worth naming. Also note the diff makes relative skills.directories / context.includeDirectories resolve against cwd at startup: equivalent today (cwd == target dir) but a startup-behavior change that rides along with the /cd work.
  • Risk: Stage 1e high-risk path match — packages/cli/src/acp-integration/acpAgent.ts and session/Session.ts (acp-integration paths correlate with post-merge reverts in this repo's history). Not a gate stop, but it means full Stage 2 enrichment and CI evidence before approval.

Moving on to code review. 🔍

中文说明

感谢贡献!

  • 模板:完整 ✓ —— 所有必需章节齐全,包含双语正文。
  • 问题:真实存在,并非理论推演。关联 issue feat(cd): Reload project-scoped runtime configuration after /cd #10173 在 issue 分诊时已在源码中核实:Config.relocateWorkingDirectory() 目前会刷新 workspace 根目录、memory 和 MCP servers,但不会重载 settings、hooks、skills 和 agent 定义——/cd 之后会话仍沿用上一个项目的 hooks 和权限/工具面。由于 hooks 会执行命令、settings 控制权限,这是真实的策略边界缺口,而非假设性问题。
  • 方向:一致。这是修正现有核心命令的语义,而不是新增产品面,且 issue 的范围约束良好(不热切换凭据/认证、不重新打开 design(serve): Define multi-workspace session cd ownership semantics #7015 的 daemon 归属问题)。找到直接行为参照:Claude Code 2.1.246 changelog——"Improved /cd: the new directory's project settings, hooks, .mcp.json servers … skills, and agents now take effect right after the move instead of on --resume"。
  • 规模:生产逻辑约 1559 行、测试约 840 行(52 个文件共 2399 行;无生成/schema 文件),横跨 packages/corepackages/clipackages/acp-bridge。超过 1000 行大 PR 建议线——如可行建议考虑拆分,但 prepare/commit/rollback 事务作为整体读起来最自然,故仅作提示。说明:两级核心门禁对维护者作者的 PR 豁免,且作者是 packages/core 的 CODEOWNER,因此不做升级——仅提示规模以保证可见性。
  • 方案:prepare-and-commit 设计符合 issue 要求:目标设置在任何状态移动之前以只读方式加载(损坏的目标配置会 fail-closed 且不改动文件),commit 在 workspace watcher 暂停期间原位替换 LoadedSettings,chdir/realpath/产物迁移失败会回滚。issue 分诊时曾建议分阶段落地(先安全相关重载,再做 skills/agents/memory 清理);本 PR 选择一次到位——鉴于共享事务这可以成立,但值得指出。另注意:diff 使相对的 skills.directories / context.includeDirectories 在启动时相对 cwd 解析——目前等价(cwd == 目标目录),但属于随 /cd 工作夹带的启动行为变化。
  • 风险:Stage 1e 高风险路径命中——packages/cli/src/acp-integration/acpAgent.tssession/Session.ts(acp-integration 路径与本仓库合并后回滚的历史相关)。不是门禁拦截,但意味着 approval 前需要完整 Stage 2 enrichment 和 CI 证据。

进入代码审查。🔍

Qwen Code · qwen3.8-max

Reviewed at 7580e80d0d51847cba55821fc401fd544c274acf · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Code review

Independent proposal first (before reading the diff): extend Config.relocateWorkingDirectory() with a prepare/commit/rollback phase driven by the CLI-owned settings loader, refresh permissions → tools → hooks → skills/subagents → memory → MCP in the new project's context, track session-owned vs project-scoped tools so the former survive the swap, fire a CwdChanged hook, and wire TUI (cdCommand) and ACP (acpAgent/Session) through the same core transaction. That is exactly what this PR does — the match is close, and it goes further in the right places: fail-closed hook reload (a failed reload drops configured hooks rather than keep running the previous project's), settings-watcher pause across the swap, and managed vs runtime-added workspace directories handled separately.

No critical blockers found. Transaction boundaries are right: prepare() loads target settings read-only and rejects before any state moves (corrupt target config throws FatalConfigError without touching the file — new test covers it); chdir/realpath/artifact-migration failures roll the settings swap back; once committed, per-subsystem refresh errors are collected as warnings instead of leaving cwd and settings diverged, which is what the issue's fail-closed spec asks for. Spot-verified against base code: recomputeMcpGating is reused from hot-reload.ts (no parallel gating logic), createToolRegistry / reinitializeMcpServers / Session.startCronScheduler / CronScheduler.destroy are pre-existing APIs, and channel sessions keep cron disabled via projectRuntimeCronEnabled (asserted in the acpAgent test).

Two follow-ups, both non-blocking:

  • Leftover process-global context-filename reads. Context filenames are threaded through the security-relevant surfaces (auto-mode protected-write checks, file exclusions, memory discovery, /memory dialog), but a few read sites still consult the process-global getCurrentGeminiMdFilename() / getAllGeminiMdFilenames(): writeContextFile.ts (serve /workspace/memory route), resolveQwenMemoryPaths in acpAgent (ACP getMemoryPaths), and serve/workspace-memory.ts. After /cd into a project with a custom context.fileName these would resolve the startup-time name. Narrow edge (custom filenames are rare; serve-managed workspaces don't relocate the same way) — fine as a follow-up.
  • Startup-behavior change riding along. Relative skills.directories and context.includeDirectories now resolve against cwd (resolveProjectSkillPath / resolveProjectPath) — needed so a target project's relative paths resolve correctly on /cd, and equivalent at startup since cwd == target dir there, but worth knowing it's in this diff. Test expectations updated accordingly.

Relocation flow

sequenceDiagram
    participant P1 as cd or ACP caller
    participant P2 as Config relocateWorkingDirectory
    participant P3 as ProjectRuntimeReloader
    participant P4 as LoadedSettings
    participant P5 as tools hooks permissions skills MCP
    P1->>P2: relocate to target dir
    P2->>P3: prepare target settings read-only
    P3-->>P2: prepared config with commit and rollback
    Note over P2,P3: prepare failure rejects before any state moves
    P2->>P4: commit settings swap, pause workspace watcher
    P2->>P2: chdir and verify realpath, roll back on mismatch
    P2->>P2: apply project runtime config
    P2->>P5: reload permissions, tools, hooks fail-closed, skills, subagents
    P2->>P5: reset and refresh memory, reinitialize MCP servers
    P2->>P5: refresh system instruction and tool declarations
    P2->>P5: fire CwdChanged hook with old and new cwd
    P2->>P3: complete, resume watcher on the new project
    P2-->>P1: success, non-blocking failures reported as warnings
Loading
Files changed (30 of 52 shown)
File What changed
packages/core/src/config/config.ts Core relocation transaction: prepare/commit/rollback wiring, applyProjectRuntimeConfig for ~50 fields, cron scheduler recycle, CwdChanged firing
packages/cli/src/config/config.ts Builds the ProjectRuntimeReloader from target-directory settings, including commit/rollback/complete with watcher pause
packages/cli/src/config/settings.ts LoadedSettings.replaceWith in-place identity swap; loadSettings gains readOnly mode (no on-disk corruption recovery during prepare)
packages/core/src/tools/tool-registry.ts Session-owned tool tracking; replaceCoreToolsFrom / clearProjectRuntimeTools / rediscoverCommandTools; explicit spawn cwd for discovered command tools
packages/core/src/hooks/hookRegistry.ts reloadConfiguredHooks gains failClosed: a failed reload drops configured hooks instead of restoring the previous project's
packages/core/src/hooks/hookSystem.ts reload options passthrough, fireCwdChangedEvent, updateHttpSecurity
packages/core/src/hooks/hookEventHandler.ts fireCwdChangedEvent with old_cwd / new_cwd payload
packages/core/src/hooks/types.ts CwdChanged event name and CwdChangedInput
packages/core/src/hooks/hookPlanner.ts Matcher-target case for CwdChanged
packages/core/src/hooks/hookRunner.ts updateHttpSecurity passthrough to the HTTP runner
packages/core/src/hooks/httpHookRunner.ts Private-network policy becomes mutable (updateSecurity)
packages/core/src/permissions/permission-manager.ts reloadForProjectChange: replaces project rules, preserves session-added rules
packages/core/src/permissions/autoMode.ts Per-config context filenames threaded through protected-write detection
packages/core/src/memory/memoryDiscovery.ts Optional contextFileNames instead of the process-global list
packages/core/src/memory/refresh.ts didWriteProjectContextFile accepts context filenames
packages/core/src/utils/workspaceContext.ts applyRootDirectories replaces managed include dirs, keeps runtime-added ones
packages/core/src/utils/ignorePatterns.ts Exclusion patterns from the config's context filenames
packages/core/src/skills/skill-manager.ts refreshForProjectChange (cache refresh + watcher retarget)
packages/core/src/subagents/subagent-manager.ts refreshForProjectChange with fail-closed project-cache drop
packages/core/src/core/coreToolScheduler.ts Passes context filenames into auto-mode review decisions
packages/cli/src/config/settingsWatcher.ts pauseWorkspaceWatching plus a processing-drain promise so pause waits for in-flight work
packages/cli/src/ui/commands/cdCommand.ts Passes trustedFolder after the trust confirmation, surfaces runtime-refresh warnings
packages/cli/src/acp-integration/acpAgent.ts ACP relocation: trust flag, refresh warnings, commands refresh, cron restart; channel sessions pin cron disabled
packages/cli/src/acp-integration/session/Session.ts Session-owned tool registration; context filenames in memory-write and auto-mode checks
packages/cli/src/ui/hooks/useGeminiStream.ts Cron scheduler restarts when the working directory changes
packages/cli/src/ui/AppContainer.tsx Context filenames from config instead of the global helper
packages/cli/src/ui/components/MemoryDialog.tsx Memory file resolution from config context filenames
packages/cli/src/ui/commands/initCommand.ts Primary context filename from config
packages/cli/src/gemini.tsx Passes LoadedSettings into loadCliConfig so the reloader exists
packages/acp-bridge/src/status.ts CwdChanged exposed in the serve hook-events surface
…and 22 more files

Test evidence

Unattended CI run — no PR code is built or executed in triage; the evidence below is the PR's own CI fetched via the API at the reviewed commit. At review time the two material checks are still running: the main Node suite (Test (ubuntu-latest, Node 22.x)) and the SDK Java daemon E2E (Real daemon E2E / Java 11). Everything completed so far is green. The macOS/Windows Node test jobs show as skipped by design — ci.yml runs them only on merge_group / schedule / workflow_dispatch, not on pull_request. No failing check to analyze. The table updates in place once CI settles.

Final CI results for 7580e80 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Test (ubuntu-latest, Node 22.x) ❌ failure
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
macos-latest / Java 21 ✅ success
Real daemon E2E / Java 11 ✅ success
Secret scan (TruffleHog) ✅ success
ubuntu-latest / Java 11 ✅ success
ubuntu-latest / Java 17 ✅ success
ubuntu-latest / Java 21 ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
windows-latest / Java 21 ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Sandboxed verification would settle this: @qwen-code /verify — the central claim is behavioural (after a live /cd, the destination project's hooks, permissions, tools, and skills become active while the previous project's are removed, and an invalid target config aborts the switch with the old runtime intact), which the diff alone cannot show and the unit suite would still pass with the reload wiring stubbed out. @qwen-code /tmux can drive the interactive /cd surface as a real user.

Real-scenario testing: not driven in this run (unattended CI path — live TUI testing is reserved for the isolated /tmux job). Not verified: live before/after /cd behaviour; the author reports macOS-only manual testing in the PR body (author's claim, not independently re-run).

中文说明

代码审查:先独立给出方案再对照 diff——本 PR 的做法与独立方案高度一致:以 prepare/commit/rollback 事务扩展 relocateWorkingDirectory,由 CLI 侧 settings loader 驱动,依次刷新权限、工具、hooks(fail-closed)、skills/子智能体、memory、MCP,并区分会话级与项目级工具,TUI 与 ACP 共用同一核心事务。未发现阻断性问题。事务边界正确:prepare 以只读方式加载目标设置、在任何状态移动前失败即拒绝(损坏配置抛 FatalConfigError 且不改动文件,有新测试覆盖);chdir/realpath/产物迁移失败会回滚;commit 之后各子系统刷新失败仅收集为警告,避免 cwd 与设置分裂——符合 issue 的 fail-closed 要求。两个非阻断跟进项:一是仍有少量读点使用进程级全局的上下文文件名(writeContextFile.ts、acpAgent 的 resolveQwenMemoryPathsserve/workspace-memory.ts),/cd 到自定义 context.fileName 的项目后这些路径会解析到启动时的文件名,边缘场景、可作后续跟进;二是相对的 skills.directories / context.includeDirectories 现在启动时相对 cwd 解析,属于为 /cd 正确性所需、启动时等价的夹带变化。上方有时序图说明迁移流程,文件总览表列出 30/52 个文件。

测试证据:本次为无人值守 CI 运行,不在分诊中构建或执行 PR 代码;证据来自 API 拉取的 PR 自身 CI。评审时两个关键检查仍在运行(ubuntu Node 22 主套件、SDK Java daemon E2E);已完成的全部通过;macOS/Windows Node 测试按 ci.yml 设计仅在 merge queue/定时/手动触发时运行,PR 上为 skipped,属预期。CI 落定后表格会原地更新。沙盒验证建议:@qwen-code /verify 可 A/B 证明目录切换后目标项目能力真正生效且旧项目资源被移除(这是 diff 与单测都无法钉住的行为性主张);@qwen-code /tmux 可驱动交互式 /cd 场景。实时 before/after 未在本次运行中执行;作者在 PR 正文中自述仅在 macOS 上人工测试(作者声明,非独立复跑)。

Qwen Code · qwen3.8-max

Reviewed at 7580e80d0d51847cba55821fc401fd544c274acf · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean review across every stage; only non-blocking follow-ups, and the approval waits on CI, not on doubt.

Stepping back: my independent proposal for this problem was, almost step for step, what this PR implements — a prepare/commit/rollback relocation transaction owned by core, driven by the CLI's settings loader, refreshing permissions → tools → hooks → skills/subagents → memory → MCP, with session-owned tools surviving the swap and both TUI and ACP on the same path. Where it goes beyond that baseline it does so in the right places: a fail-closed hook reload (a broken reload drops configured hooks rather than silently keep executing the previous project's), the settings watcher paused across the swap so the old directory is never watched mid-transition, and managed include directories replaced while runtime-added ones survive. The test suite (~840 lines) pins exactly where this kind of change leaks: apply/reject/rollback paths, session-tool preservation, permission-rule replacement with session rules intact, the CwdChanged payload, cron restart on relocation. The problem itself was source-verified during issue triage, and the direction has a direct external reference (Claude Code 2.1.246 shipped the same /cd semantics).

What keeps this at 4 rather than 5: a handful of read sites still consult the process-global context filename instead of the per-config one (serve memory route, ACP getMemoryPaths — noted in the review comment, fine as a follow-up), and ~1559 production lines across three packages is a large unit to land even when the transaction reads coherently. Neither blocks.

CI status: the main Node suite and the daemon E2E were still running at review time, so no approval is posted in this run. Approval is deferred until CI lands green on the reviewed commit; the marker below carries it.

中文说明

置信度:4/5 —— 各阶段审查均干净;只有非阻断的跟进项,等待的是 CI 而不是因为存疑。

退一步看:我对这个问题的独立方案与本 PR 几乎逐步一致——由 core 拥有、CLI settings loader 驱动的 prepare/commit/rollback 迁移事务,依次刷新权限 → 工具 → hooks → skills/子智能体 → memory → MCP,会话级工具在切换中保留,TUI 与 ACP 走同一路径。超出基线的部分也都用在正确的地方:hooks 以 fail-closed 方式重载(重载失败时丢弃已配置 hooks,而不是悄悄继续执行上一个项目的)、切换期间暂停设置监听以避免过渡期仍监听旧目录、受管 include 目录被替换而运行时新增目录得以保留。测试套件(约 840 行)恰好钉住了这类改动最容易泄漏的位置:应用/拒绝/回滚路径、会话工具保留、权限规则替换且会话规则不受影响、CwdChanged 载荷、迁移后 cron 重启。问题本身已在 issue 分诊时于源码中核实,方向也有直接外部参照(Claude Code 2.1.246 已上线相同的 /cd 语义)。

之所以是 4 而不是 5:少数读点仍读取进程级全局上下文文件名而非按配置的(serve memory 路由、ACP getMemoryPaths——已在审查评论中说明,可作后续跟进);约 1559 行生产逻辑横跨三个包,即便事务读起来连贯,仍是较大的落地单元。两者都不阻断。

CI 状态:评审时主 Node 套件与 daemon E2E 仍在运行,因此本次不发布 approve。批准延迟到 CI 在评审的 commit 上全绿;由下方标记承接。

Qwen Code · qwen3.8-max

Reviewed at 7580e80d0d51847cba55821fc401fd544c274acf · re-run with @qwen-code /triage

@qqqys

qqqys commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Aug 27, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

中文说明

仅完成部分审查,审查缺口已披露。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

— qwen3.8-max via Qwen Code /review (v0.22.2)

Comment thread packages/cli/src/config/config.ts Outdated
Comment thread packages/cli/src/config/config.ts Outdated
Comment thread packages/cli/src/config/config.ts Outdated
Comment thread packages/cli/src/config/config.ts Outdated
Comment thread packages/cli/src/config/settings.ts
Comment thread packages/core/src/config/config.ts
Comment thread packages/core/src/config/config.ts
Comment thread packages/core/src/permissions/permission-manager.ts
Comment thread packages/core/src/subagents/subagent-manager.ts
Comment thread packages/core/src/utils/workspaceContext.ts
@qqqys

qqqys commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover stop

@qwen-code-dev-bot qwen-code-dev-bot removed the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Aug 27, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply autofix/takeover (or comment @qwen-code /takeover) to re-engage.

中文说明

👋 已释放:autofix 循环不再介入此 PR(在飞的一轮如有,将完成其有界工作)。重新打上 autofix/takeover 标签(或评论 @qwen-code /takeover)即可再次接管。

qqqys added 2 commits August 28, 2026 01:38
Resolves the context-filename conflicts by keeping this branch's session-scoped
`config.getContextFileNames()` design and adopting main's renamed globals
(`getAllMemoryFilenames` / `setMemoryFilename` / `memoryFileCount`) as the
fallbacks. Test mocks of memory-constants export both spellings.
…e reload

The eleven Critical findings, plus the Suggestions that describe runtime
behaviour rather than coverage alone.

Critical:
- R1-1  `commit()` now applies the target's `.env`/`settings.env` to
        `process.env` (`reloadEnvironment`, as serve's workspaceReload does)
        and `rollback()` restores the previous directory's. `prepare()`
        takes the directory being left for that; the core interface gained
        the parameter.
- R1-2  A bare session's `/cd` projects from `createMinimalSettings()`
        instead of loading the real user-scope files.
- R1-3  `resolveProjectSkillPath` expands home spellings first, so
        `%userprofile%` skill directories survive (they were nailed under
        the project).
- R1-4  Permission-rule persistence never routes through a minimal
        LoadedSettings (`resolvePersistenceSettings`).
- R1-5  `reloadScopeFromDisk` re-runs the migration for a scope that was
        migrated in memory, so the first hot-reload after the move no longer
        regresses to the legacy layout.
- R1-6  Non-string `context.fileName` entries are dropped instead of throwing
        out of a half-committed relocation; the apply step is also wrapped so
        `complete()` (which resumes the watcher) is always reached.
- R1-7  Session-scoped context names threaded through the remaining
        consumers: `/directory add`, the TUI memory refresh, the ACP
        `getMemoryPaths` request (answered from the session owning the cwd),
        and optional parameters on the two daemon memory helpers.
- R1-8  `applyProjectRuntimeConfig` clears the legacy `hooks` fallback so a
        hook-less target cannot revive the previous project's hooks.
- R1-9  The CwdChanged fire site checks `getDisableAllHooks()` like every
        other fire site.
- R1-10 `clearProjectRuntimeTools` removes command-discovered tools only;
        core tools and factories survive a failed refresh.
- R1-11 Command discovery skips names the session owns.

Suggestions with a runtime effect:
- R1-14 the reloader replicates startup's `tool_search` denial (explicit
        setting, or the session model captured once at startup)
- R1-15 `agents` goes through the same projection startup uses
        (`projectAgentsSettings`), so `team.*` no longer appears after /cd
- R1-17 the resumed workspace watcher reconciles against disk
- R1-23 the cron work the swap cancelled is reported (`cronExitSummary`)
        by both consumers instead of vanishing with the destroyed scheduler
- R1-26 runtime-added directories are not absorbed into the managed set
        when an intermediate project happens to list them

Coverage the review asked for: R1-12 (session-owned registration), R1-13
(a real-file reloader suite), R1-16, R1-18, R1-19, R1-20, R1-22 (both
rollback sites), R1-24. Left as-is: R1-21 (the wider settings-parity audit
is a separate change) and R1-25 (refresh helpers' own unit tests).

Mutation-verified in three batches, 19 reverts against the full affected
suites: each revert reddens exactly its own test(s) and nothing else.

Claude-Session: https://claude.ai/code/session_01VXsC4f71S6U6YkW82NRw7m
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head 6d1bdd6, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

No response changes against the PR base across 12 scenario(s).

Qwen Code · serve A/B

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": verifying the settings merge layer ( mergeSettings in packages/cli/src/config/settings.ts) passes a non-array security.allowedHttpHookUrls value through to …; "agent reverse-audit (round 2)": checking whether resolveDisabledSlashCommands and the remaining applyProjectRuntimeConfig spreads share the same unguarded shape.

Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:

  • packages/cli/src/config/config.ts:1922 — [probe] complete() success-path watcher resume is never executed by any test
  • packages/cli/src/config/config.ts:1708 — [probe] cronEnabledOverride wired but never exercised through the real reloader
  • packages/core/src/config/config.test.ts:242 (+3 locations) — [review] Three relocate call sites unpinned: reload failClosed, tool-swap catch, client refresh
  • packages/cli/src/config/settings.ts:757 — [review] Target-project settings warnings collected on /cd but never surfaced
中文说明

仅完成部分审查,审查缺口已披露。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 2)"verifying the settings merge layer ( mergeSettings in packages/cli/src/config/settings.ts) passes a non-array security.allowedHttpHookUrls value through to …"agent reverse-audit (round 2)"checking whether resolveDisabledSlashCommands and the remaining applyProjectRuntimeConfig spreads share the same unguarded shape

收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.2)

Comment thread packages/core/src/config/config.ts
Comment thread packages/core/src/config/config.ts
Comment thread packages/core/src/config/config.ts Outdated
Comment thread packages/cli/src/config/config.ts Outdated
Comment thread packages/cli/src/config/config.ts Outdated
Comment thread packages/core/src/config/config.ts
Comment thread packages/core/src/memory/writeContextFile.ts Outdated
Comment thread packages/core/src/memory/writeContextFile.ts Outdated
Comment thread packages/core/src/subagents/subagent-manager.ts
Comment thread packages/core/src/skills/skill-manager.ts
@qqqys

qqqys commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

已修复本轮 5 条 Critical:目录切换后会同步后台智能体并发限制、worktree 软链设置和权限持久化目标,safe mode 保留用户级 agents 设置;非法 allowedHttpHookUrls 会安全降级且不再中断后续运行时配置应用。验证通过:全仓 build、typecheck;core 747 项相关测试、CLI 366 项相关测试;目标 ESLint、Prettier 与 git diff --check。修复提交 afbf358

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

@qqqys

qqqys commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /review

@github-actions

Copy link
Copy Markdown
Contributor

Qwen Code review request accepted. Review is queued in workflow run.

@qqqys

qqqys commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

已修复 upstream 合并后的编译回归:项目运行时刷新改用当前 LlmClient 实例,消除 core 中两处 geminiClient 不存在错误。

验证:Node 22 下仓库 build、typecheck 通过;core config.test.ts 599/599;目标 ESLint、Prettier 与独立 relocation 路径验证通过。提交 6d1bdd6

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • R2-18 bare-mode persistence fallback re-enters loadSettings(cwd) with env loading enabled — already reported (comment 3875850392)
  • cronEnabledOverride never exercised through the real reloader (test gap) — already recorded in the round-2 deferral list (review 5045650144)
  • complete() success-path watcher resume never executed by any test — already recorded in the round-2 deferral list (review 5045650144)
  • tool-swap catch / reload failClosed / client-refresh call sites unpinned — already recorded in the round-2 deferral list (review 5045650144)

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Convergence: round 3 posted 26 inline comment(s), 15 of them reported for the first time; the previous round posted 21 (18 new). Findings keep coming back to the same files: packages/cli/src/config/config.ts (findings in rounds 1, 2; 4 more now); packages/core/src/config/config.ts (findings in rounds 1, 2; 1 more now); packages/cli/src/config/config.projectRuntimeReloader.test.ts (findings in round 2; 1 more now), and 1 more file(s). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

仅完成部分审查,审查缺口已披露。

本轮确认的 4 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

收敛情况:第 3 轮发布了 26 条行内评论,其中 15 条是首次提出;上一轮发布了 21 条(其中 18 条首次提出)。发现反复回到同一批文件:packages/cli/src/config/config.ts(第 1、2 轮已出过发现,本轮又有 4 条);packages/core/src/config/config.ts(第 1、2 轮已出过发现,本轮又有 1 条);packages/cli/src/config/config.projectRuntimeReloader.test.ts(第 2 轮已出过发现,本轮又有 1 条),另有 1 个文件。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)

— qwen3.8-max via Qwen Code /review (v0.22.2)

Comment thread packages/cli/src/config/config.ts Outdated
Comment thread packages/cli/src/config/config.ts
Comment thread packages/core/src/config/config.test.ts
Comment thread packages/core/src/config/config.test.ts
Comment thread packages/core/src/config/config.test.ts
Comment thread packages/core/src/memory/writeContextFile.ts Outdated
Comment thread packages/cli/src/acp-integration/session/Session.ts
Comment thread packages/core/src/config/config.ts
Comment thread packages/core/src/subagents/subagent-manager.ts
Comment thread packages/core/src/skills/skill-manager.ts
qqqys added 3 commits August 29, 2026 19:38
…on process ownership

The "Always allow" persistence callback read the rule list from the
session's in-memory LoadedSettings. In --safe-mode the target loads with
skipWorkspaceSettings, so that scope is empty in memory while the file on
disk is not: the first persist after /cd overwrote the project's existing
allow rules. In normal mode it replayed a stale list against a sibling
session's write. Every persist now read-modify-writes a fresh, side-effect
free (skipLoadEnvironment) disk load of the target directory, which is what
startup did before the callback was extracted.

commit() rewrote process.env from a per-session /cd. An ACP child under
`qwen serve` hosts every session on its channel and spawned MCP servers and
shell tools inherit process.env, so a sibling session's subprocesses picked
up this project's secrets while losing their own. The reloader now takes a
host policy with ownsProcessEnvironment(); the ACP host answers false while
sibling sessions are live, commit() leaves process.env alone and reports a
warning through projectRuntimeRefreshErrors, and rollback() only restores an
environment it actually replaced. QWEN_DISABLED_SLASH_COMMANDS is resolved
from the target project's environment view during prepare() instead of from
the pre-commit process.env.
…untime reload

- tool-registry: a rediscovered command tool can no longer replace a
  built-in (core factory or eager tool); only fresh names register
- settingsWatcher: promote/demote re-check the scope generation after
  their async close, so a pause that landed mid-close cannot be undone by
  the stale continuation re-arming the previous project's `.qwen`
- settings: a readOnly load no longer stamps `$version` into memory
- acpAgent: `contextFileNamesForCwd` normalizes both sides before
  comparing, so a trailing-slash or `..` spelling still resolves the
  session's context-file names
- agents projection: carry the schema-declared arena limits; document
  why `team` (schema-reserved, no keys) is not carried
- background-tasks: the constructor delegates to setConcurrencyLimits so
  construction and `/cd` reload share one validation path
- drop the dead `contextFileName` / `contextFileNames` parameters on
  writeWorkspaceContextFile and collectWorkspaceMemoryStatus (no caller);
  threading those through the serve routes is follow-up work
- drop the duplicate resolveProjectSkillPath helper
- comments: cron effect deps in use-llm-stream, cronRecurringMaxAgeDays
- tests: reloader reentrancy guards, complete() resume, host cron policy,
  per-model cap replacement, session-owned create_sub_session pin,
  prepare's previous-dir argument, include-dir memory and trust
  re-application, readOnly `$version`, ACP scoped memory paths, watcher
  in-flight-promote race, built-in collision gate
@qqqys

qqqys commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

本轮已推送 3 个提交(32c6d5a 合并 upstream/main、85c6931、1da1167),回复并关闭了全部 49 条未解决线程:

  • 合并 mainfix(core): decouple permissions.allow from tool registration via tools.eager (#10075) #10098permissions.allow 的注册门改成了 tools.eager,与本 PR 的 tool 重载语义冲突。已迁移:ProjectRuntimeConfig.eagerTools 替代 permissions.registryAllowList,启动与 /cd 共用 resolveEagerTools() / Config.normalizeEagerTools()PermissionManager.reloadForProjectChange 适配新字段。
  • R1-4(第 3 次)根因:持久化回调不再读内存快照,每次都基于目标目录的新磁盘加载做 read-modify-write(回到提取回调前启动路径的做法),同时关掉 R2-18。
  • R3-1:新增 ProjectRuntimeHostPolicy.ownsProcessEnvironment();ACP 子进程存在兄弟会话时 /cd 不改写 process.env,通过 projectRuntimeRefreshErrors 报告警告,rollback() 只恢复它实际替换过的环境。QWEN_DISABLED_SLASH_COMMANDS 改为在 prepare() 中按目标项目的环境视图解析(R2-20)。
  • Suggestion:R2-2/3/4/5(部分)/6/7/9/10/19/26、R1-12、R3-2~R3-9 已修复;无调用者的 contextFileName(s) 参数已删除。
  • 延后:R1-21、R1-25、R2-5 的 team、serve 路由的上下文文件名贯通、按会话 spawn 环境 → feat(cd): follow-ups to the /cd project runtime reload (#10263) #10502

验证(Node 22):全仓 npm run buildnpm run typecheck;core 825 + CLI 1842 项相关测试通过;改动文件 ESLint / Prettier / git diff --check 通过;R3-1、R2-20、R3-8、R3-9 四处修复均做了突变检查(改坏后对应测试变红)。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • refreshForProjectChange (SubagentManager/SkillManager) direct-test gap — already reported as R1-25/R3-14 in rounds 1-3 and deferred by the author to follow-up issue #10502 item 5

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not reviewed: build-and-test — local unit suites did not complete (runner time budget exhausted during workspace builds; packages/cli typecheck incomplete; CI Test (ubuntu-latest, Node 22.x) is failing at this commit).

Not explored to full depth (tool budget reached): chunk 17: executing the three touched test files ( refresh.test.ts , autoMode.test.ts , permission-manager.test.ts ) — the review worktree has no node_modules , and in….

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 3.

Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:

  • packages/cli/src/config/settings.ts:654 — [review] hot reload of a legacy settings file reverts ui.theme (normalization skipped)
  • packages/cli/src/config/settings.ts:654 — [review] readOnly /cd path drops migration warnings
  • packages/core/src/config/config.ts:6128 — [review] /cd into a hooks-enabled project leaves a hook-disabled startup's hook subsystem dead
  • packages/core/src/config/config.ts:981 — [review] /cd hard-fails on a missing include directory that startup skips
  • packages/cli/src/ui/components/MemoryDialog.test.tsx:130 — [review] MemoryDialog tests cannot see the session-scoped switch (vacuous pairing)
  • packages/core/src/core/coreToolScheduler.ts:2801 — [review] AUTO-mode review call sites pass session context-file names unwitnessed
  • packages/cli/src/acp-integration/acpAgent.ts:10274 — [review] ACP /cd warning surfacing (refresh errors, cron summary) unwitnessed
  • packages/cli/src/acp-integration/session/Session.ts:3270 — [review] speak_to_user / live-task session-owned registration unwitnessed
  • packages/cli/src/acp-integration/session/Session.ts:10204 (+2 locations) — [review] didWriteProjectContextFile session-name argument unwitnessed (2 call sites)
  • packages/core/src/config/config.test.ts:7305 — [review] commit-failure test never asserts complete() stays uncalled
  • packages/core/src/tools/tool-registry.ts:590 — [review] failed /cd tool refresh can orphan permissionDeferred entries
  • packages/core/src/config/config.test.ts:7184 — [review] { failClosed: true } argument of the /cd hook reload is unpinned
  • packages/cli/src/config/config.projectRuntimeReloader.test.ts:234 — [review] reloader's bare/safe-mode disableAllHooks gate unwitnessed
  • packages/cli/src/ui/commands/initCommand.ts:36 — [review] /init session-scoped filename swap unwitnessed

Convergence: round 4 posted 10 inline comment(s), 10 of them reported for the first time; the previous round posted 26 (15 new). Findings keep coming back to the same files: packages/cli/src/acp-integration/acpAgent.ts (findings in round 2; 3 more now); packages/cli/src/config/config.ts (findings in rounds 1, 2, 3; 2 more now); packages/cli/src/config/config.projectRuntimeReloader.test.ts (findings in round 3; 1 more now), and 3 more file(s). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

仅完成部分审查,审查缺口已披露。

本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

未审查:build-and-test — local unit suites did not complete (runner time budget exhausted during workspace builds; packages/cli typecheck incomplete; CI Test (ubuntu-latest, Node 22.x) is failing at this commit)。

未探索到全部深度(达到工具调用预算):chunk 17:executing the three touched test files ( refresh.test.ts , autoMode.test.ts , permission-manager.test.ts ) — the review worktree has no node_modules , and in…

未审查:反向审计——在 3 轮的反审轮数上限内未收敛。

收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 14 条(原文未翻译,列表见上方英文部分)。

收敛情况:第 4 轮发布了 10 条行内评论,其中 10 条是首次提出;上一轮发布了 26 条(其中 15 条首次提出)。发现反复回到同一批文件:packages/cli/src/acp-integration/acpAgent.ts(第 2 轮已出过发现,本轮又有 3 条);packages/cli/src/config/config.ts(第 1、2、3 轮已出过发现,本轮又有 2 条);packages/cli/src/config/config.projectRuntimeReloader.test.ts(第 3 轮已出过发现,本轮又有 1 条),另有 3 个文件。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)

— qwen3.8-max via Qwen Code /review (v0.22.3)

Comment on lines +12592 to +12595
// This child hosts every session on its channel and spawned tools
// inherit `process.env`, so a per-session `/cd` may only rewrite
// the process environment while no sibling session is live.
ownsProcessEnvironment: () => this.sessions.size <= 1,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R3-1: (fix-induced) [certifies-falsely] [new-surface] The round-3 fix for R3-1 gated the /cd process.env rewrite on ownsProcessEnvironment: () => this.sessions.size <= 1, but the predicate counts only STORED sessions. A session mid-creation is invisible in this.sessions until createAndStoreSession stores it AFTER the awaited llmClient.initialize() (acpAgent.ts:12935), and initializingConfigs is only populated when chatRecording !== false — so a /cd committed while a sibling session/new is in flight still rewrites process.env.

An ACP child hosts session A; the host issues session/new for B, whose creation awaits llmClient.initialize() (auth + MCP startup — seconds). The host issues session/cd for A inside that window: ownsProcessEnvironment() reads sessions.size === 1 → true → commit() applies A's target-project env to process.env and strips the previous project's file-sourced keys. B's own env keys (applied at its session/new start) are wiped and never re-applied, and every MCP server and shell tool B spawns afterwards inherits A's project environment — the exact cross-session leakage this gate exists to prevent. Re-evaluating the predicate at commit time does not close this: an unstored creation is invisible at any evaluation point.

Witness:

witness: not run — closest capability was an interleaving probe in the acpAgent.test.ts harness (stall session B inside llmClient.initialize, issue qwen/control/session/cd for A, assert the env rewrite ran); not seeded because it requires the full ACP connection + session-lifecycle mock scaffolding, while the guard's blindness is a static fact — the closure reads this.sessions.size only, and sessions.set is structurally after the awaited init.

Fix: count in-flight creations in the predicate — increment a pending-creation counter at newSessionConfig entry, decrement it on store and on every failure path, and use this.sessions.size + pendingSessionCreations <= 1. initializingConfigs alone cannot serve as that counter (acpAgent.ts:12621-12622 — populated only when chatRecording !== false).

Fix witness: in acpAgent.test.ts, hold a creation in flight (mock llmClient.initialize with a deferred promise), start session/new, capture the host-policy argument passed to loadCliConfig and assert ownsProcessEnvironment() returns false — removing the in-flight count must turn it red.

中文说明

第 3 轮 R3-1 的修复把 /cdprocess.env 的改写门控为 ownsProcessEnvironment: () => this.sessions.size <= 1,但该谓词只统计已存储的会话。创建中的会话在 createAndStoreSessionawait llmClient.initialize()(acpAgent.ts:12935)之后才进入 this.sessions,而 initializingConfigs 仅在 chatRecording !== false 时填充——因此在兄弟 session/new 创建过程中提交的 /cd 仍会改写 process.env

具体场景:ACP 子进程承载会话 A;host 发起 session/new 创建 B,B 的创建正等待 llmClient.initialize()(鉴权 + MCP 启动,耗时数秒);此时 host 对 A 发起 session/cdownsProcessEnvironment() 读到 sessions.size === 1 → true → commit() 把 A 的目标项目环境写入 process.env 并删除上一个项目的文件来源键。B 自己的环境键(在其 session/new 启动时已应用)被清除且不会重新应用,B 之后派生的所有 MCP 服务器与 shell 工具都会继承 A 的项目环境——正是该门控要防止的跨会话泄漏。把谓词推迟到 commit 时求值也无法闭合此问题:未存储的创建在任何求值点都不可见。

修复:在谓词中计入创建中的会话——在 newSessionConfig 入口递增一个待创建计数器,在存储时及每个失败路径递减,使用 this.sessions.size + pendingSessionCreations <= 1initializingConfigs 本身不能充当该计数器(仅在 chatRecording !== false 时填充)。

修复见证:在 acpAgent.test.ts 中挂起一个创建中的会话(用延迟 promise mock llmClient.initialize),发起 session/new,捕获传给 loadCliConfig 的 host-policy 参数并断言 ownsProcessEnvironment() 返回 false——移除创建中计数后该测试应变红。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Comment on lines +284 to +285
reloadForProjectChange(): void {
if (this.strippedAllowRules?.session.length) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R4-17: [certifies-falsely] [regression] reloadForProjectChange() unconditionally clears the AUTO-mode dangerous-rule stash and relies on initialize() to re-strip, but initialize() only re-strips when the BASE config's approval mode is 'auto' (permission-manager.ts:231-233; the PM's config is the base Config — config.ts:3567). A strip acquired by a subagent's AUTO override (InProcessBackend.acquireAutoApprovalOverride, InProcessBackend.ts:522) while the base session stays non-AUTO is silently dropped by /cd, re-activating the dangerous allow rules under the still-running AUTO subagent. On agent exit restoreDangerousRules() is a no-op (stash undefined), so the lost state is never repaired. Pre-diff, nothing except the paired restoreDangerousRules ever cleared the stash.

Concrete shape: trusted folder; base session in DEFAULT mode with a dangerous allow rule (Bash); an AUTO subagent runs with the strip active. The user /cds → the stash is cleared, initialize() sees base mode ≠ 'auto' → no re-strip. The AUTO agent now gets 'allow' for everything the broad rule matches, bypassing the AUTO classifier/manual review for the rest of its run (shouldForceAutoModeReviewForAllow only intercepts protected-write paths).

Witness:

probe (real PermissionManager, base mode 'default', rule Bash):
PR code:  stash after reload: undefined / decision after reload: allow / decision after agent exit: allow
with fix: stash after reload: {persistent:[Bash],session:[]} / decision after reload: ask / decision after agent exit: allow

Fix:

const wasStripped = this.strippedAllowRules !== undefined;
// ... existing restore + reset + initialize() ...
if (wasStripped && !this.strippedAllowRules) {
  this.stripDangerousRulesForAutoMode();
}

The re-strip must leave the stash defined so release-time restore still works: stripDangerousRulesForAutoMode early-returns when already defined (permission-manager.ts:1449-1451), and releaseAutoApprovalOverride restores only when base mode is not AUTO (InProcessBackend.ts:541-545).

Fix witness: permission-manager.test.ts — config with approvalMode: 'default' and permissionsAllow: ['Bash']; initialize(); stripDangerousRulesForAutoMode(); reloadForProjectChange(); assert the stash is still defined and the dangerous command does not evaluate to 'allow'. Removing the re-strip guard turns it red.

中文说明

reloadForProjectChange() 会无条件清除 AUTO 模式危险规则暂存(stash)并依赖 initialize() 重新剥离,但 initialize() 仅在基础 Config 的审批模式为 'auto' 时才重新剥离(permission-manager.ts:231-233;PM 持有的是基础 Config,config.ts:3567)。当基础会话保持非 AUTO、而子代理通过 AUTO 覆盖(InProcessBackend.acquireAutoApprovalOverride,InProcessBackend.ts:522)获得的剥离状态,会被 /cd 静默丢弃,危险 allow 规则在仍在运行的 AUTO 子代理下重新生效。代理退出时 restoreDangerousRules() 因 stash 为 undefined 而空操作,丢失的状态永远不会被修复。改动前除了成对的 restoreDangerousRules 之外没有任何路径会清除该 stash。

证据(真实 PermissionManager,基础模式 'default',规则 Bash):PR 代码下 reload 后 stash 为 undefined、决策为 allow;按建议修复后 reload 后 stash 保留、决策为 ask,双向翻转验证通过。

修复:在重置前捕获 wasStrippedinitialize() 之后若 wasStripped && !this.strippedAllowRules 则补一次 stripDangerousRulesForAutoMode();重剥离须保持 stash 已定义,以免破坏释放时的恢复路径。

修复见证:在 permission-manager.test.ts 中以 approvalMode: 'default' + permissionsAllow: ['Bash'] 构造,依次 initialize()stripDangerousRulesForAutoMode()reloadForProjectChange(),断言 stash 仍有定义且危险命令不为 'allow';移除补剥离守卫后应变红。

— qwen3.8-max via Qwen Code /review (v0.22.3)

webSearch:
bareMode || safeMode
? undefined
: resolveWebSearchSettings(runtimeSettings),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R4-22: [certifies-falsely] [new-surface] prepare() resolves webSearch against the OLD project's process.env: resolveWebSearchSettings reads ENABLE_WEB_SEARCH/WEB_SEARCH_MODEL/WEB_SEARCH_EXTRACTOR/WEB_SEARCH_BASE_URL/WEB_SEARCH_API_KEY from process.env (cli config.ts:1030-1043), but the target's env only reaches process.env later, in commit()'s reloadEnvironment. The sibling resolveDisabledSlashCommands is explicitly passed targetEnvironment for exactly this reason (config.ts:1555-1561); webSearch is not.

Session in project A whose .env sets WEB_SEARCH_BASE_URL=https://a.example/search; /cd to project B whose .env sets its own base URL and WEB_SEARCH_API_KEY=key-B. prepare() snapshots A's baseUrl into the config; commit() rewrites process.env to B's values; at call time the web-search tool sends the request to the captured baseUrl with the bearer key read live from process.env (web-search.ts:220-221) — B's API key is transmitted to A's endpoint. Inverse: A's ENABLE_WEB_SEARCH=false keeps web search disabled after /cd into a B that enables it; in a shared process (ownsProcessEnvironment false) the host daemon's env is consulted instead of the computed target view.

Witness:

probe (real reloader, real .env files):
after prepare(projectB): webSearch = { enabled: true, baseUrl: 'https://a.example/search' }
after commit():          process.env.WEB_SEARCH_BASE_URL = 'https://b.example/search'
with fix (env param fed targetEnvironment): baseUrl becomes 'https://b.example/search'; inverse arm flips to enabled true; shared-process arm resolves the target view
Suggested change
: resolveWebSearchSettings(runtimeSettings),
: resolveWebSearchSettings(runtimeSettings, targetEnvironment),

Add an env: Readonly<NodeJS.ProcessEnv> = process.env parameter to resolveWebSearchSettings (same shape as resolveDisabledSlashCommands) and pass targetEnvironment here. The API key itself is read at invocation time from process.env[apiKeyEnv] (packages/core/src/tools/web-search.ts:221), so only the env var NAME and literal values (baseUrl, model, enabled) may be captured from the target view.

Fix witness: a case in config.projectRuntimeReloader.test.ts that puts WEB_SEARCH_BASE_URL values into process.env differing from the target's env view and asserts prepared.config.webSearch.baseUrl equals the target view; removing the new env argument keeps the suite green and reddens this test.

中文说明

prepare() 用的是旧项目process.env 来解析 webSearchresolveWebSearchSettings 直接从 process.env 读取上述 5 个环境变量(cli config.ts:1030-1043),而目标项目的环境要到 commit()reloadEnvironment 才写入 process.env。兄弟函数 resolveDisabledSlashCommands 正是因为这个原因被显式传入 targetEnvironmentwebSearch 却没有。

场景:项目 A 的 .env 设置 WEB_SEARCH_BASE_URL=https://a.example/search/cd.env 中设置了自己的 URL 与 WEB_SEARCH_API_KEY=key-B 的项目 B。prepare() 把 A 的 baseUrl 快照进配置;commit()process.env 改写为 B 的值;调用时 web-search 工具向捕获的 baseUrl 发请求、并从 process.env 实时读取密钥(web-search.ts:220-221)——B 的 API 密钥被发送到 A 的端点。反向场景:A 的 ENABLE_WEB_SEARCH=false 会在 /cd 到启用 web search 的 B 后继续保持禁用;共享进程(ownsProcessEnvironment 为 false)下则完全读取宿主环境而非计算出的目标视图。

修复:给 resolveWebSearchSettings 增加 env 参数(与 resolveDisabledSlashCommands 同形),此处传入 targetEnvironment。密钥本身在调用时从 process.env[apiKeyEnv] 读取,因此只应从目标视图捕获变量名与字面值。

修复见证:在 config.projectRuntimeReloader.test.ts 中令 process.env 与目标环境视图的 WEB_SEARCH_BASE_URL 不同,断言 prepared.config.webSearch.baseUrl 等于目标视图;移除新参数后该测试变红而原套件仍绿。

— qwen3.8-max via Qwen Code /review (v0.22.3)

skipWorkspaceSettings: safeMode,
workspaceTrusted: trustedFolder,
});
const effectiveTrust = trustedFolder ?? nextSettings.isTrusted;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R4-23: [fails-closed] [regression] In bare mode nextSettings is createMinimalSettings(), which hardcodes isTrusted: false (settings.ts:698-716), so an unconfirmed /cd in a bare session flips Config.trustedFolder from true to false. Startup computes isWorkspaceTrusted(settings)?.isTrusted ?? true (cli config.ts:1986), which returns { isTrusted: true } when folder trust is disabled (trustedFolders.ts:410-417) — bare sessions START trusted.

qwen --bare interactive session; /cd ../other. cdCommand skips the trust branch (isFolderTrustEnabled({}) is false) and passes no trustedFolder, so prepare() computes effectiveTrust = undefined ?? false = false and applyProjectRuntimeConfig installs it. From then on setApprovalMode(AUTO|AUTO_EDIT|YOLO) throws TrustGateError (core config.ts:7479-7484) — a session that started trusted and could switch modes before the move cannot after it, with no UI path to re-trust the folder (folder trust is disabled in bare mode), and getTrustedDerivedApprovalMode silently downgrades derived/subagent configs.

Witness:

probe:
isWorkspaceTrusted(createMinimalSettings().merged)?.isTrusted ?? true === true   (startup)
prepared.config.trustedFolder === false                                          (bare prepare, no trustedFolder arg)
createMinimalSettings().isTrusted === false
with fix: prepared.config.trustedFolder === true
Suggested change
const effectiveTrust = trustedFolder ?? nextSettings.isTrusted;
const effectiveTrust = trustedFolder ?? (bareMode ? (isWorkspaceTrusted(nextSettings.merged)?.isTrusted ?? true) : nextSettings.isTrusted);

An explicit confirmation must still win — cdCommand passes { trustedFolder: true } only after the confirm round-trip.

Fix witness: a bare-mode case in config.projectRuntimeReloader.test.ts asserting prepared.config.trustedFolder === true from prepare(targetDir) with no trustedFolder argument; reverting to trustedFolder ?? nextSettings.isTrusted reddens it.

中文说明

bare 模式下 nextSettingscreateMinimalSettings(),其 isTrusted 硬编码为 false(settings.ts:698-716),因此 bare 会话中未经确认的 /cd 会把 Config.trustedFolder 从 true 翻转为 false。而启动时按 isWorkspaceTrusted(settings)?.isTrusted ?? true 计算(cli config.ts:1986),在文件夹信任未启用时返回 { isTrusted: true }(trustedFolders.ts:410-417)——bare 会话启动时是受信任的。

场景:qwen --bare 交互式会话执行 /cd ../othercdCommand 跳过信任分支且未传 trustedFolderprepare() 得到 effectiveTrust = false 并由 applyProjectRuntimeConfig 写入。此后 setApprovalMode(AUTO|AUTO_EDIT|YOLO)TrustGateError——一个启动时受信任、移动前可以自由切换模式的会话,移动后再也不能切换,且 bare 模式下没有 UI 路径重新信任目录;派生/子代理配置也会被 getTrustedDerivedApprovalMode 静默降级。

修复:在 bare 分支镜像启动逻辑(见 suggestion 代码块);显式确认仍须优先——cdCommand 只在确认往返之后才传 { trustedFolder: true }

修复见证:在 config.projectRuntimeReloader.test.ts 增加 bare 模式用例,断言不带 trustedFolder 参数的 prepare(targetDir) 返回 trustedFolder === true;还原为 trustedFolder ?? nextSettings.isTrusted 后变红。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Comment on lines +654 to +656
if (this.migratedInMemoryScopes.has(scope) && needsMigration(parsed)) {
parsed = runMigrations(parsed, scope).settings;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] R4-24: [certifies-falsely] [new-surface] After a readOnly /cd load (migration in memory, disk left legacy by design), the first workspace-scope setValue patches the still-legacy disk file in MERGE mode (updateSettingsFilePreservingFormat preserves keys not in updates), and this hot-reload migration branch then resolves the resulting legacy+nested collision in favor of the LEGACY value — silently reverting the user's write.

/cd into a project whose .qwen/settings.json is V1-shaped {"theme": "dark"}; the readOnly load migrates in memory and the disk stays legacy. The user changes theme to light: setValuesaveSettings deep-merges into the disk file, keeping the legacy top-level key ({"theme":"dark","ui":{"theme":"light"}}). The watcher fires and reloadScopeFromDisk(Workspace) hits this branch; v1→v2 writes the legacy value into ui.theme first (v1-to-v2.ts:162-191) and skips the colliding nested child via wasProcessed (v1-to-v2.ts:225-247), so ui.theme resolves to legacy 'dark' — the user's 'light' is silently dropped ~300ms after they set it. On the next startup the same migration persists the reverted value permanently. Pre-diff this could not occur: a project's settings file was always migrated on disk at first load.

Witness:

probe (real code):
disk starts {"theme":"dark"}; readOnly load → merged.ui.theme 'dark', migratedInMemoryScopes.has(Workspace) true
setValue(Workspace,'ui.theme','light') → disk {"theme":"dark","ui":{"theme":"light"}}
reloadScopeFromDisk(Workspace) → merged.ui.theme === 'dark'   ← user's 'light' silently dropped
removing the migration branch → merged.ui.theme stays 'light' (branch is load-bearing for the revert)

Fix: persist the in-memory migration once the relocation is committed — in the reloader's commit()/complete(), write file.originalSettings for each scope in migratedInMemoryScopes via a sync-mode updateSettingsFilePreservingFormat (or make the first post-/cd write of such a scope a sync-mode write of the full migrated object) — so legacy keys never coexist with their migrated counterparts on disk. The persist belongs at/after commit, never inside the readOnly load (the no-write-before-commit contract stated by settings.project-runtime.test.ts:98-100).

Fix witness: settings.project-runtime.test.ts — readOnly-load legacy {"theme":"dark"}, replaceWith, run the post-commit persist step, setValue(Workspace,'ui.theme','light'), reloadScopeFromDisk(Workspace), assert merged.ui?.theme === 'light'; removing the persist step reddens it.

中文说明

readOnly 的 /cd 加载只在内存中迁移、按设计不落盘之后,第一次工作区作用域的 setValue 会以合并模式修补仍是旧版格式的磁盘文件(updateSettingsFilePreservingFormat 保留更新中未提及的键),而这个热重载迁移分支随后会把「旧顶层键 + 新嵌套键」的冲突解析为旧值——静默回滚用户刚写入的值。

场景:/cd 进入 .qwen/settings.json 为 V1 格式 {"theme": "dark"} 的项目;readOnly 加载在内存中迁移,磁盘保持旧格式。用户把主题改为 light:磁盘变为 {"theme":"dark","ui":{"theme":"light"}};watcher 触发 reloadScopeFromDisk(Workspace) 走到该分支,v1→v2 先把旧顶层值写入 ui.theme、再经 wasProcessed 跳过冲突的嵌套子项,ui.theme 变回 'dark'——用户设置的 'light' 在约 300ms 后被静默丢弃;下次启动时同一迁移会把回滚后的值永久落盘。改动前不会出现此问题:项目的设置文件首次加载时就已在磁盘上完成迁移。

修复:在迁移提交后把内存迁移持久化——在 reloader 的 commit()/complete() 中对 migratedInMemoryScopes 里的每个作用域以同步模式写入 file.originalSettings(或让该作用域在 /cd 后的首次写入以同步模式写入完整迁移后对象),使旧键与其迁移后的键不再同时存在于磁盘。持久化必须在 commit 时/之后,不能在 readOnly 加载内(settings.project-runtime.test.ts:98-100 声明的 commit 前不写盘契约)。

修复见证:在 settings.project-runtime.test.ts 中 readOnly 加载旧格式文件、replaceWith、执行 commit 后持久化步骤、setValue 改主题、reloadScopeFromDisk,断言 merged.ui?.theme === 'light';移除持久化步骤后变红。

— qwen3.8-max via Qwen Code /review (v0.22.3)

const requested = path.resolve(cwd);
for (const session of this.sessions.values()) {
const config = session.getConfig();
if (path.resolve(config.getWorkingDir()) === requested) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R4-4: contextFileNamesForCwd normalizes both sides with path.resolve only, but after /cd the session's stored working directory is a REALPATH (core config.ts stores expectedCanonicalDir ?? fs.realpathSync(targetPath)), while the host-supplied param is not — so a symlinked (or case-folded) spelling of the very same directory misses every session and falls through to the stale process-global filename list. This is a sibling corner of the fixed R2-2 (the trailing-slash/raw-spelling case, closed by path.resolve in 1da1167).

A session /cds into /tmp/proj; core stores /private/tmp/proj (macOS realpath). The host calls qwen/settings/getMemoryPaths with cwd: '/tmp/proj': path.resolve('/tmp/proj') !== '/private/tmp/proj' → no session matches → the fallback getAllMemoryFilenames() returns the process-global list; the host gets QWEN.md paths for a project whose session loaded CONTEXT.md, and the host's memory editor reads/writes a file the session never loads, silently discarding user edits.

Witness:

probe (real symlink in the acpAgent.test.ts harness, session getWorkingDir() = realpath, host request via the link spelling):
PR code: PROJECT-MEMORY-FILE = <link>/QWEN.md   ← global fallback, not the session's CONTEXT.md
with realpath canonicalization of both sides: PROJECT-MEMORY-FILE = <link>/CONTEXT.md (and the trailing-slash case still passes)

Fix: canonicalize both sides with realpath before comparing, e.g.

const canonical = (p: string): string => {
  try {
    return fs.realpathSync(p);
  } catch {
    return path.resolve(p);
  }
};

and compare canonical(config.getWorkingDir()) === canonical(cwd). Both sides must be canonicalized: a session that never /cd-ed stores a resolved-but-not-realpathed directory (see the comment above this block), so realpathing only the requested side would break the construction-time match.

Fix witness: extend the scoped-session case in acpAgent.test.ts (~12252): create a symlink to the real directory, have the fake session's getWorkingDir() return the realpath, query getMemoryPaths with the link spelling, and assert CONTEXT.md is returned; removing the realpath normalization reddens it.

中文说明

contextFileNamesForCwd 两侧都只用 path.resolve 归一化,但 /cd 之后会话存储的工作目录是 REALPATH(core config.ts 存储 expectedCanonicalDir ?? fs.realpathSync(targetPath)),而宿主提供的参数不是——同一目录的符号链接(或大小写折叠)写法会匹配不到任何会话,落入过时的进程全局文件名列表。这是已修复的 R2-2(尾斜杠/原始写法,1da1167 用 path.resolve 闭合)的兄弟角落。

修复:比较前用 realpath 规范化两侧(失败时回退 path.resolve);两侧都必须规范化——从未 /cd 的会话存储的是 resolve 但非 realpath 的目录,只规范化请求侧会破坏构造时的匹配。

修复见证:扩展 acpAgent.test.ts 的 scoped-session 用例:创建符号链接、让会话 getWorkingDir() 返回 realpath、以链接写法查询 getMemoryPaths,断言返回 CONTEXT.md;移除 realpath 规范化后变红。

— qwen3.8-max via Qwen Code /review (v0.22.3)

// This child hosts every session on its channel and spawned tools
// inherit `process.env`, so a per-session `/cd` may only rewrite
// the process environment while no sibling session is live.
ownsProcessEnvironment: () => this.sessions.size <= 1,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R4-6: the cross-session env-leak guard ownsProcessEnvironment: () => this.sessions.size <= 1 is wired with NO test anywhere exercising the predicate (zero ownsProcessEnvironment references in acpAgent.test.ts). The reloader-level tests supply () => false directly, so mutating the agent wiring — or dropping the field entirely, which the reloader defaults to true (cli config.ts:1511) — ships with a fully green suite.

Mutation <= 1true: an ACP child hosting two live sessions; session 1 /cds and rewrites process.env; session 2's subsequently spawned MCP servers and shell tools inherit session 1's project secrets and lose session 2's own env keys — exactly the leak the predicate exists to prevent.

Witness:

probe: mutate the wiring to ownsProcessEnvironment: () => true, run acpAgent.test.ts
junit: tests="517" failures="0" errors="0"   (baseline also 517/517 — the mutation survives)

Fix: capture the loadCliConfig host-policy argument in an acpAgent relocation/host-policy test and assert ownsProcessEnvironment() is true with one live session and false once a second entry is added to agent.sessions. The test goes red if <= 1 becomes <= 0, >= 1, or true. (Pairs with the R3-1 fix above, whose own witness covers the in-flight-creation arm.)

中文说明

跨会话环境泄漏守卫 ownsProcessEnvironment: () => this.sessions.size <= 1 的接线没有任何测试覆盖(acpAgent.test.ts 中 ownsProcessEnvironment 零引用)。reloader 层的测试直接提供 () => false,因此修改该接线——甚至整个删除该字段(reloader 默认 true,cli config.ts:1511)——都会在全套件绿灯下合入。

修复:在 acpAgent 的重定位/host-policy 测试中捕获 loadCliConfig 的 host-policy 参数,断言仅一个活跃会话时 ownsProcessEnvironment()trueagent.sessions 加入第二个后为 false<= 1 变为 <= 0/>= 1/true 时变红。(与上方 R3-1 修复配套,其见证测试覆盖创建中会话的场景。)

— qwen3.8-max via Qwen Code /review (v0.22.3)

// tearing down a scheduler that immediately restarts. The effect should
// run once on mount and clean up only on real unmount.
// tearing down a scheduler that immediately restarts. The effect DOES
// list `cronWorkingDir`: a `/cd` must stop the previous project's

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R4-14: the effect cleanup's freedom from a duplicate "loops cancelled" notice depends on relocateWorkingDirectory destroying the scheduler BEFORE committing the new working directory (core config.ts:6071-6075), and no test pins that ordering. The new cron-restart test mocks getExitSummary: () => null, so the cleanup's stderr write can never fire there; core tests pin only cronExitSummary capture.

Mutation: move this.cronScheduler?.destroy() past the commit point (e.g. below await refreshCurrentRuntimeStatus). The post-/cd re-render's cleanup then reads a non-null getExitSummary() from the not-yet-destroyed scheduler and process.stderr.writes the raw multi-line cancellation notice into the Ink-rendered TUI, while cdCommand simultaneously surfaces the same text via formatCronRelocationNotice — duplicated, UI-garbling output. Every existing test stays green.

Witness:

witness: not run — closest capability was a mutation probe moving cronScheduler.destroy() past the commit point and driving the cron-effect harness with a non-null exit summary; not run because the existing effect harness mocks getExitSummary: () => null by construction (a new effect-level fixture would be required) — the substance (no test pins the ordering/silence) is settled by the test inventory above.

Fix: add a relocation test that exercises the real ordering — give the old scheduler a session-only job, run relocateWorkingDirectory, and assert the exit summary surfaces exactly once (via the returned cronExitSummary) and the effect cleanup's stderr path stays silent. The summary must stay captured before destroy() clears jobs/wakeups (cronScheduler.ts:1623); reordering destroy after the cwd commit must turn the test red.

中文说明

effect cleanup 之所以不会打印重复的 "loops cancelled" 通知,依赖 relocateWorkingDirectory 在提交新工作目录之前销毁调度器(core config.ts:6071-6075),而没有任何测试钉住该顺序。新的 cron 重启测试把 getExitSummary mock 为 () => null,cleanup 的 stderr 写入在那里永远不可能触发。

变异:把 destroy() 移到提交点之后——/cd 后重渲染的 cleanup 会从未销毁的调度器读到非空退出摘要,把原始多行通知写进 Ink TUI,同时 cdCommand 又经 formatCronRelocationNotice 输出同样内容——重复且扰乱界面,而所有现有测试仍为绿。

修复:新增走真实顺序的重定位测试——旧调度器带一个仅会话级任务,执行 relocateWorkingDirectory,断言退出摘要只通过返回的 cronExitSummary 出现一次、cleanup 的 stderr 路径保持静默;把 destroy 重排到 cwd 提交之后应变红。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Comment on lines +404 to +406
await prepared.complete();
await prepared.complete();
expect(resume).toHaveBeenCalledOnce();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R4-28: complete()'s appEvents.emit(AppEvent.McpPendingApprovalChanged) (cli config.ts:1845-1849) is unwitnessed: this doubled-complete test calls complete() twice without spying on appEvents.emit, while the rollback twin's emission IS asserted in the swap test (line 390); the only test reaching committed complete() has no emit spy.

Delete the appEvents.emit(…) line from complete() and the entire suite stays green; after a successful /cd the target project's pending MCP approval gating (assembled in prepare) is never re-signaled to the UI, which keeps showing the previous project's approval state until an unrelated event fires.

Witness:

probe:
mutant deleting the emit from complete() → Tests 16 passed (16)   (mutation invisible)
mutant + suggested spy/assertion        → 1 failed
intact code + assertion                 → Tests 16 passed (16)

Fix: in 'resumes the watcher exactly once when the switch completes', vi.spyOn(appEvents, 'emit') and assert it was called with AppEvent.McpPendingApprovalChanged after complete(), mirroring the rollback assertion in the swap test. Removing the emit from complete() makes the new assertion red.

中文说明

complete()appEvents.emit(AppEvent.McpPendingApprovalChanged)(cli config.ts:1845-1849)没有见证:这个双调用 complete() 的测试没有对 appEvents.emit 建 spy,而 swap 测试中 rollback 的同一事件是有断言的(第 390 行)。

删除 complete() 里的 emit,全套件仍绿;成功的 /cd 之后,目标项目的待批准 MCP 门控不会再通知 UI,界面会一直显示旧项目的审批状态直到无关事件触发。

修复:在 'resumes the watcher exactly once when the switch completes' 中对 appEvents.emit 建 spy 并断言 complete() 后以 AppEvent.McpPendingApprovalChanged 被调用;移除 emit 后该断言变红。

— qwen3.8-max via Qwen Code /review (v0.22.3)

}

/**
* Replaces both concurrency caps wholesale — the per-model map is NOT

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] R4-30: the /cd wiring of maxConcurrentBackgroundAgentsByModel is unwitnessed: applyProjectRuntimeConfig passes both caps (core config.ts:5804-5808), but the relocation test asserts only the global cap (config.test.ts:7988-8022); the by-model startup describe goes through the constructor and the registry-level tests call setConcurrencyLimits directly. normalizePerModelConcurrency(undefined) returns an empty map, so dropping the line is invisible.

Delete the maxConcurrentBackgroundAgentsByModel line from applyProjectRuntimeConfig: setConcurrencyLimits receives undefined, the per-model map becomes empty, and the entire suite stays green — /cd into a project configuring agents.maxParallelAgentsByModel then silently enforces NO per-model throttling (a model capped at 1 can spawn unbounded background agents), diverging from a fresh session in the same project.

Witness:

probe:
baseline over config.test.ts + background-tasks.test.ts → Tests 743 passed (743)
mutant deleting the byModel line                        → Tests 743 passed (743)   (invisible)
mutant + suggested relocation extension                 → expected true to be false
intact code + extension                                 → 1 passed

Fix: extend 'relocateWorkingDirectory updates agent, worktree, and persistence runtime state' to prepare the target with agents: { maxParallelAgents: 3, maxParallelAgentsByModel: { 'weak-model': 1 } }, register one running weak-model agent, and assert canStartBackgroundAgent('weak-model') is false after relocation. Removing the byModel wiring line reddens it.

中文说明

/cdmaxConcurrentBackgroundAgentsByModel 的接线没有见证:applyProjectRuntimeConfig 同时传入两个上限(core config.ts:5804-5808),但重定位测试只断言全局上限;按模型上限在启动路径走构造函数、在 registry 层测试中直接调用 setConcurrencyLimitsnormalizePerModelConcurrency(undefined) 返回空映射,删除该行完全不可见。

删除该行后:/cd 进入配置了 agents.maxParallelAgentsByModel 的项目将静默不执行任何按模型限流(上限为 1 的模型可派生无限后台代理),与全新会话行为分叉,而全套件仍绿(743/743)。

修复:扩展重定位测试——目标配置 maxParallelAgentsByModel: { 'weak-model': 1 },注册一个运行中的 weak-model 代理,断言重定位后 canStartBackgroundAgent('weak-model')false;删除接线行后变红。

— qwen3.8-max via Qwen Code /review (v0.22.3)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/self-reported The linked issue was opened by the PR author (self-reported)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cd): Reload project-scoped runtime configuration after /cd

3 participants